-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
support transparency #18
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
=========================================
Coverage ? 41.19%
=========================================
Files ? 10
Lines ? 352
Branches ? 0
=========================================
Hits ? 145
Misses ? 207
Partials ? 0 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed it.
Would you mind adding a test for this?
I couldn't find a way to fake |
Why would you need the interactive mode to run the test? I mean, it's more or less like how Sixel.jl/test/backend/libsixel.jl Lines 60 to 63 in 07011d3
RGBA , right?
img = rand(RGBA, 5, 5)
io = IOBuffer()
sixel_encode(io, img)
@test ref == String(take!(io)) |
I got confused by Line 9 in 07011d3
|
It's okay to use |
Last commit is optional, let me know I you don't want it. |
Let's put this on hold since the real fix would (hopefully) be brought by libsixel/libsixel#23. I'm planning of bumping |
@t-bltg Perhaps we should work together instead of separately, as I got a bit scared off by all the memory safety issues in libsixel so have been afraid to make big changes to the C code, especially because we know for a fact the current API's aren't even able to be made safe as a few of them lack critical information like data size. I don't know Julia, sorry, but my longterm plan upstream has been to:
|
That's certainly a good idea to transition to |
Fix #17.
PR (mlterm)
master